-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
treewide: remove with lib in meta, will be dismembered later #371665
base: master
Are you sure you want to change the base?
Conversation
Please, bot. Do not tag everyone. |
083ec14
to
d737646
Compare
d737646
to
2d07585
Compare
6a8821e
to
deb4639
Compare
557aa4d
to
1a2d582
Compare
9035276
to
b0f0f58
Compare
b0f0f58
to
1c880b4
Compare
Signed-off-by: lucasew <[email protected]>
Signed-off-by: lucasew <[email protected]>
Signed-off-by: lucasew <[email protected]>
What even is the problem? My impression is that people religiously chase every single Because to be clear, writing out |
with lib.maintainers; [ ... ] is fine. The problem is open scoped withs. I removed some of those actually but I shouldn't. This PR will not get merged. It's just a starting point for a migration where someone brings a subset of it's diff and tweak stuff if necessary. |
I am working on adding static checks on CI. This is me hyperfocusing too soon, and studying about the problem and it's different nuances. |
Things done
Remove with lib in meta, proactively, in the whole repo, once and for all
Methodology
Basically, all packages with a
with lib
in meta will havemeta = with lib;
in it,so I just replaced treewide this string to
meta =
in all**/*.nix
.This naive alteration would break a lot of stuff, like everything, so I had to fix the
broken stuff.
For this there is nil which is a code checker and also
implements the language server protocol. Then I did a simple bash for that iterates over
**/*.nix
and applies the LSP to it discarding the output. Files with failure then arelisted and saved in a
nil-failures.log
. Why log? Log is in the gitignore so no chanceof causing a mess. At least from this part.
To start fixing what is basically thousands and thousands of files, I used
substituteInPlace
with some common patterns, not overthinking too much to catch them all. Examples:
license = licenses
→license = lib.licenses
maintainers = with maintainers
→maintainers = with lib.maintainers
And many others, basically following this same pattern of simple text substitution, no regex.
Between the substitutions, I ran that nil loop again to recalculate the failing files and then
decided the next replacement.
After about a dozen iterations, there remained about 800 files to be fixed. The following files
were not changed because of a false positive where __curPos is undefined, but it seemed like an
implementation detail:
lib/tests/modules/merge-module-with-key.nix
lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix
lib/tests/modules/types-attrTag.nix
nixos/modules/profiles/nix-builder-vm.nix
pkgs/applications/emulators/wine/sources.nix
pkgs/development/compilers/corretto/mk-corretto.nix
pkgs/top-level/release-cross.nix
By consequence, this PR also fixes issues where
version
is not defined inmeta.changelog
, whichin some cases I had to rewrite some
mkDerivation
using thefinalAttrs
form. There were rare cases,about 10, probably.
Also by consequence, nil warned about unused
rec
s and unused arguments, which I removed. Also, there wereunused let variables, which I didn't touch. A remarkable case was a arch something in some ROCm files.
There were some rare cases where passthru were defined after meta, then I moved it up.
TL;DR: this shouldn't have any rebuilds or eval issues.
Review tips
Download the full PR patch and look through the diff from there. Most of the time the diff will be boringly the same.
I'll squash the commits when this is about to get merged
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.